-
Notifications
You must be signed in to change notification settings - Fork 326
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: register gRPC Gateway routes #2838
fix: register gRPC Gateway routes #2838
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution! LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can go ahead and approve these since we have not gathered user data on who actually needs the rest adapters. If there are not many users, then it might make sense to disable them entirely to focus on only supporting the canonical grpc ones
Last approval is remaining from @rootulp. Remaining CI checks haven't run yet but i expect there won't be any issue with that. |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2838 +/- ##
==========================================
- Coverage 19.70% 19.16% -0.54%
==========================================
Files 139 142 +3
Lines 17005 17291 +286
==========================================
- Hits 3350 3313 -37
- Misses 13333 13676 +343
+ Partials 322 302 -20 ☔ View full report in Codecov by Sentry. |
The CI test has failed due to the |
Congrats, your important contribution to this open-source project has earned you a GitPOAP! GitPOAP: 2023 Celestia Contributor: Head to gitpoap.io & connect your GitHub account to mint! Learn more about GitPOAPs here. |
<!-- Please read and fill out this form before submitting your PR. Please make sure you have reviewed our contributors guide before submitting your first PR. --> ## Overview <!-- Please provide an explanation of the PR, including the appropriate context, background, goal, and rationale. If there is an issue with this information, please provide a tl;dr and link the issue. --> This PR fixes #2837. Both `blob` and `blobstream` modules lack registration for the gRPC Gateway routes, leading to the inability to query endpoints. Let me know if there is anything i need to do to merge this PR. CI checks require 3 workflows awaiting approval. ## Checklist <!-- Please complete the checklist to ensure that the PR is ready to be reviewed. IMPORTANT: PRs should be left in Draft until the below checklist is completed. --> - [ ] New and updated code has appropriate documentation - [ ] New and updated code has new and/or updated testing - [ ] Required CI checks are passing - [ ] Visual proof for any user facing features like CLI or documentation updates - [x] Linked issues closed with keywords (cherry picked from commit b75c7e3)
This is an automatic backport of pull request #2838 done by [Mergify](https://mergify.com). --- <details> <summary>Mergify commands and options</summary> <br /> More conditions and actions can be found in the [documentation](https://docs.mergify.com/). You can also trigger Mergify actions by commenting on this pull request: - `@Mergifyio refresh` will re-evaluate the rules - `@Mergifyio rebase` will rebase this PR on its base branch - `@Mergifyio update` will merge the base branch into this PR - `@Mergifyio backport <destination>` will backport this PR on `<destination>` branch Additionally, on Mergify [dashboard](https://dashboard.mergify.com) you can: - look at your merge queues - generate the Mergify configuration with the config editor. Finally, you can contact us on https://mergify.com </details> Co-authored-by: JayB <[email protected]>
Overview
This PR fixes #2837.
Both
blob
andblobstream
modules lack registration for the gRPC Gateway routes, leading to the inability to query endpoints. Let me know if there is anything i need to do to merge this PR.CI checks require 3 workflows awaiting approval.
Checklist